:seedling: Add uninstall feature test by perdasilva · Pull Request #2453 · operator-framework/operator-controller

perdasilva · 2026-01-20T17:16:17Z

Description

Updates e2e suite to add the uninstall feature with two tests:

cluster extension resource deletion leads to resource removal
cluster extension resource deletion leads to resource removal even if the cluster extension service account is not present

Updates the ClusterExtension is rolled out step to collect the cluster extension resources and attach them to the test's context

The following steps are added:

ClusterExtension resources are created and labeled: checks that the extension resources in the context exist and carry the appropriate olm owner-kind and owner-name labels
ClusterExtension is removed: deletes the ClusterExtension under test
ClusterExtension resources are removed: checks that the extension resources in the context (eventually) no longer exist on the cluster
resource <some resource> is eventually not found: checks that the resource is eventually not on the cluster

The extension resources are acquired from either the Helm release secret or the latest active ClusterExtensionRevision for the ClusterExtension depending on the feature-gate settings.

Note: OLM's internal helm client uses a custom release backend that can handle arbitrarily large bundles. In the e2e implementation, getting the helm release resources does not yet support sharded releases because:

it goes a little beyond the intention of this PR
its not needed for the current test suite
Boxcutter does not yet support arbitrarily large bundles -> e2e's should be updated for this case when we work on that story

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

netlify · 2026-01-20T17:16:38Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`a3f4ae0`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/697b81cf8d42530008000f65
😎 Deploy Preview	https://deploy-preview-2453--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copilot

Pull request overview

This PR adds an end-to-end test for verifying that bundle resources are properly removed when a ClusterExtension is uninstalled. The test uses the Gherkin BDD format to define the uninstall scenario.

Changes:

Added new uninstall.feature file with a test scenario that verifies resource cleanup after ClusterExtension deletion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/features/uninstall.feature

codecov · 2026-01-20T17:33:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (dbc4af3) to head (a3f4ae0).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2453      +/-   ##
==========================================
- Coverage   74.02%   73.52%   -0.50%     
==========================================
  Files         101      102       +1     
  Lines        7991     8231     +240     
==========================================
+ Hits         5915     6052     +137     
- Misses       1626     1718      +92     
- Partials      450      461      +11

Flag	Coverage Δ
e2e	`46.92% <ø> (-0.19%)`	⬇️
experimental-e2e	`54.34% <ø> (-0.38%)`	⬇️
unit	`57.55% <ø> (-0.11%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/steps/steps.go

camilamacedo86

Just I nit otherwise
/approved

perdasilva · 2026-01-21T12:52:22Z

/hold to add another test

perdasilva · 2026-01-21T14:08:32Z

/unhold

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/steps/steps.go

camilamacedo86 · 2026-01-28T15:01:59Z

/lgtm cancel

I think we need a second person doing reviews too :-)

tmshort · 2026-01-28T15:10:25Z

/lgtm cancel

I think we need a second person doing reviews too :-)

I was ok with it, but @pedjak needed to check his concerns were addressed.

pedjak · 2026-01-28T17:59:06Z

test/e2e/features/uninstall.feature

+
+  Scenario: Removing ClusterExtension triggers the extension uninstall, eventually removing all installed resources
+    When ClusterExtension is removed
+    Then ClusterExtension is uninstalled


we should be here more verbose, i.e. to replace "uninstalled" with what actually happens:

Suggested change

Then ClusterExtension is uninstalled

Then resources being part of ClusterExtension are removed

Would "ClusterExtension resources are removed" work as well?

Would "ClusterExtension resources are removed" work as well?

hm, not sure. When I read it like that I would understand it as resources of type ClusterExtension are removed. What we actually assert that all resources owned by the removed ClusterExtension are removed too.

What about "the ClusterExtension's constituent resources are removed"?

pedjak · 2026-01-28T18:00:51Z

test/e2e/steps/steps.go

+			kind := obj.GetObjectKind().GroupVersionKind().Kind
+			clusterObj, err := getResource(kind, obj.GetName(), obj.GetNamespace())
+			if err != nil {
+				godog.Logf(ctx, "error getting resource name=%q namespace=%q kind=%q: %v", obj.GetName(), obj.GetNamespace(), kind, err)


use logger instance.

pedjak · 2026-01-28T18:02:50Z

test/e2e/steps/steps.go

+			if labels["olm.operatorframework.io/owner-kind"] != "ClusterExtension" {
+				godog.Logf(ctx, "resource name=%q namespace=%q kind=%q has bad owner-kind label %q (expected=ClusterExtension)", obj.GetName(), obj.GetNamespace(), kind, labels["olm.operatorframework.io/owner-kind"])
+				return false
+			}
+			if labels["olm.operatorframework.io/owner-name"] != sc.clusterExtensionName {


nit: perhaps you can declare a map with labels you want to check and then have a for loop over them?

pedjak · 2026-01-28T18:04:23Z

test/e2e/steps/steps.go

+		return fmt.Errorf("resource %s is not in the format <kind>/<name>", resource)
+	}
+
+	require.Eventually(godog.T(ctx), func() bool {


use waitFor method available in this file already.

pedjak · 2026-01-28T18:06:12Z

test/e2e/features/install.feature

-    And resource "networkpolicy/test-operator-network-policy" is installed
-    And resource "configmap/test-configmap" is installed
-    And resource "deployment/test-operator" is installed


I would keep the verbose assertions here, because it can gives us better signal when the test fails.

pedjak · 2026-01-28T18:06:23Z

test/e2e/features/install.feature

-    And resource "networkpolicy/test-operator-network-policy" is installed
-    And resource "configmap/test-configmap" is installed
-    And resource "deployment/test-operator" is installed


same as above.

pedjak · 2026-01-28T18:46:15Z

test/e2e/steps/steps.go

 	"github.com/cucumber/godog"
 	jsonpatch "github.com/evanphx/json-patch"
-	"github.com/google/go-cmp/cmp"
+	diff "github.com/google/go-cmp/cmp"


not sure if we really need this diff alias, IMHO cmp is clear?

good catch - I needed to make that change in an earlier iteration where we imported the standard cmp library!

test/e2e/steps/hooks.go

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/steps/steps.go

test/e2e/features/uninstall.feature

test/e2e/steps/hooks.go

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/e2e/steps/steps.go

test/e2e/steps/hooks.go

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

pedjak

/lgtm

openshift-ci · 2026-01-29T15:57:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, pedjak, tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [camilamacedo86,pedjak,tmshort]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot AI review requested due to automatic review settings January 20, 2026 17:16

openshift-ci bot requested review from bentito and pedjak January 20, 2026 17:16

Copilot started reviewing on behalf of perdasilva January 20, 2026 17:16 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

test/e2e/features/uninstall.feature Outdated Show resolved Hide resolved

test/e2e/features/uninstall.feature Outdated Show resolved Hide resolved

test/e2e/features/uninstall.feature Outdated Show resolved Hide resolved

test/e2e/features/uninstall.feature Outdated Show resolved Hide resolved

perdasilva marked this pull request as draft January 20, 2026 18:27

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 20, 2026

perdasilva force-pushed the uninstall-e2e-test branch from d7180c9 to 255a7c0 Compare January 20, 2026 18:51

perdasilva marked this pull request as ready for review January 21, 2026 07:28

Copilot AI review requested due to automatic review settings January 21, 2026 07:28

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 21, 2026

openshift-ci bot requested review from grokspawn and trgeiger January 21, 2026 07:28

Copilot started reviewing on behalf of perdasilva January 21, 2026 07:28 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

test/e2e/steps/steps.go Outdated Show resolved Hide resolved

perdasilva changed the title ~~🌱 Add unintall feature test~~ 🌱 Add uninstall feature test Jan 21, 2026

perdasilva force-pushed the uninstall-e2e-test branch from 255a7c0 to 1bd6544 Compare January 21, 2026 09:56

camilamacedo86 reviewed Jan 21, 2026

View reviewed changes

test/e2e/steps/steps.go Outdated Show resolved Hide resolved

camilamacedo86 reviewed Jan 21, 2026

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2026

Copilot AI review requested due to automatic review settings January 21, 2026 14:08

perdasilva force-pushed the uninstall-e2e-test branch from 1bd6544 to 1b10dc6 Compare January 21, 2026 14:08

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2026

Copilot started reviewing on behalf of perdasilva January 21, 2026 14:08 View session

Copilot AI reviewed Jan 21, 2026

View reviewed changes

test/e2e/steps/steps.go Show resolved Hide resolved

test/e2e/steps/steps.go Show resolved Hide resolved

test/e2e/steps/steps.go Show resolved Hide resolved

perdasilva force-pushed the uninstall-e2e-test branch from 1b10dc6 to 63cb602 Compare January 21, 2026 14:53

openshift-ci bot assigned camilamacedo86 Jan 28, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2026

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2026

pedjak reviewed Jan 28, 2026

View reviewed changes

test/e2e/steps/hooks.go Show resolved Hide resolved

Copilot AI review requested due to automatic review settings January 29, 2026 11:10

perdasilva force-pushed the uninstall-e2e-test branch from 89e8b11 to 8172d3a Compare January 29, 2026 11:10

Copilot started reviewing on behalf of perdasilva January 29, 2026 11:11 View session

perdasilva force-pushed the uninstall-e2e-test branch from 8172d3a to 13de323 Compare January 29, 2026 11:12

Copilot AI reviewed Jan 29, 2026

View reviewed changes

test/e2e/steps/steps.go Show resolved Hide resolved

test/e2e/steps/steps.go Show resolved Hide resolved

test/e2e/features/uninstall.feature Show resolved Hide resolved

pedjak reviewed Jan 29, 2026

View reviewed changes

test/e2e/steps/hooks.go Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings January 29, 2026 12:22

perdasilva force-pushed the uninstall-e2e-test branch from ffbf321 to acd8577 Compare January 29, 2026 12:22

Copilot started reviewing on behalf of perdasilva January 29, 2026 12:23 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

test/e2e/steps/steps.go Outdated Show resolved Hide resolved

test/e2e/steps/hooks.go Outdated Show resolved Hide resolved

perdasilva force-pushed the uninstall-e2e-test branch 2 times, most recently from 425545d to 34dcd55 Compare January 29, 2026 15:36

Add unintall feature test

a3f4ae0

Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Copilot AI review requested due to automatic review settings January 29, 2026 15:50

perdasilva force-pushed the uninstall-e2e-test branch from 34dcd55 to a3f4ae0 Compare January 29, 2026 15:50

Copilot started reviewing on behalf of perdasilva January 29, 2026 15:51 View session

pedjak approved these changes Jan 29, 2026

View reviewed changes

openshift-ci bot assigned pedjak Jan 29, 2026

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 29, 2026

Copilot AI reviewed Jan 29, 2026

View reviewed changes

openshift-merge-bot bot merged commit fbe909f into operator-framework:main Jan 29, 2026
40 of 41 checks passed

	Then ClusterExtension is uninstalled
	Then resources being part of ClusterExtension are removed

Conversation

perdasilva commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Reviewer Checklist

Uh oh!

netlify bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

perdasilva commented Jan 21, 2026

Uh oh!

perdasilva commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

camilamacedo86 commented Jan 28, 2026

Uh oh!

tmshort commented Jan 28, 2026

Uh oh!

pedjak Jan 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

perdasilva commented Jan 20, 2026 •

edited

Loading

netlify bot commented Jan 20, 2026 •

edited

Loading

codecov bot commented Jan 20, 2026 •

edited

Loading

pedjak Jan 28, 2026 •

edited

Loading